fix(admin): name the access lists, and stop confirming presses that did nothing - #213

Screenshots in the request showed two things wrong with /admin/access-lists: rows identified by a number the admin has no use for, and an add control parked as an aside on a section head. Fixing the first turned up a third problem underneath it — two controls that confirmed work they had not done.

The name

getWatchedListViews read names off the drift snapshot, which only exists once the list has been read. A list added to the watchlist a minute ago rendered as #4001 — while accessListCatalog, the table the <select> is built from, held its name the whole time.

It now LEFT JOINs the catalog and COALESCEs catalog name over snapshot name. That ordering is not arbitrary: it matches what watchedListName() already does for audit rows (src/services/access-lists.ts:93-104), so the audit log and the page cannot disagree about what a list is called.

The two presses that lied

addWatch uses onConflictDoNothing; removeWatch deletes by id. Both are idempotent, and both actions read "no error" as "it worked":

• Adding a list another admin had just added → "List added to the watchlist."
• Removing a row another tab had already removed → "Fleet staging removed from the watchlist."

Neither press wrote an audit row, so the notice was the only account of what happened, and it credited this admin with an act that had already happened without them.

Neither is reachable by clicking alone — the <select> never offers a watched list, and a removed row takes its button with it. Both are reachable exactly the way a real admin reaches them: the page renders, the table changes underneath it, and the press lands against a world that has moved.

Both services now return whether they changed anything. The two actions differ in how they can say so:

• addWatchAction redirects, and ConfirmNotice carries a sentence and no tone, so its correction is wording alone — ?done=watch-already → "That list was already on the watchlist. Nothing was added."
• removeWatchAction deliberately does not redirect (a redirect resets the Disclosure state), so it returns a warn ActionOutcome and gets both the words and the tone.

I did not add a tone channel to ConfirmNotice to make these symmetric — it is shared with /account and /admin/sync, and that is a wider change than this task asked for.

The silent refusal

useSubmitGuard refuses a second press while the first is in flight. Where the first press navigates, silence is right. Here it is not: removeWatchAction does not redirect, so the page does not move and the admin watches a press do nothing. Worse, the whole region shares one ConfirmingForm, so the button refused is frequently a different row's than the one in flight.

StopWatching moved out of page.tsx into its own "use client" file to wire onRefused. That is the whole reason for the new file: onRefused is a function, and a server component cannot hand a function to a client one.

Layout

The add control became a field group above the list rather than an aside on the section head — it acts on the collection, not on any row, and "Add to watchlist" is the thing an admin does repeatedly, where "Designate as holder" is done once. The <select> is required with a disabled placeholder, which also fixes a real defect: an untouched submit used to POST with no accessListId and throw invalid_id.

The page adopted .page__head/.page__lede, which let the duplicate .lede rule leave globals.css. monitorSentence's "normal" case gained a real sentence naming the holder and stating what the page compares — deliberately without claiming reads are healthy, since monitorState never sees a readStatus and warn rows render directly beneath that sentence.

Verification

npm test              92 files, 1477 tests passed
npm run typecheck     clean
npm run lint          clean
npm run format:check  All matched files use Prettier code style!
npm run build         succeeded
npx playwright test e2e/access-lists.spec.ts    15 passed

Both new e2e tests were mutation-proved: forcing addWatchAction to always emit done=watch and deleting the if (!removed) branch failed exactly those two tests and nothing else (13 passed). Both mutations were reversed and the restore verified against the diff.

code-reviewer ran clean — admin guard on all four actions, audit writes confined to the branches that actually change state, enqueue-don't-execute boundary untouched, no orphaned comment claims.

Where to look

src/services/access-lists.ts — the two return-value changes are the substance; everything else follows from them. The docblocks there argue why "idempotent" and "confirmed as done" are different claims, which is the point of the whole change.

Summary by CodeRabbit

• New Features

  • Added a catalog-based control for selecting access lists to monitor.
  • Added clear confirmations when monitoring starts, including when a list is already monitored.
  • Added named controls for stopping monitoring.
  • Improved monitoring descriptions to clarify roster comparisons and list status.

• Bug Fixes

  • Prevented empty or duplicate monitoring submissions.
  • Added warnings for attempts to stop monitoring lists that are no longer active.
  • Improved access-list names shown in monitoring views and activity records.